Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: allow auto releasing to pypi #154

Closed
wants to merge 5 commits into from

Conversation

aqeelat
Copy link
Collaborator

@aqeelat aqeelat commented Oct 17, 2023

No description provided.

python -m twine check dist/*

- name: Publish package distributions to PyPI
if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags')
Copy link
Member

@nicoddemus nicoddemus Oct 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this if is not necessary, given the trigger is already push on tags?

on:
  push:
    tags:
    - '*'

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied it from the default Jazzband template. I'll read more about it to confirm.
https://github.com/jazzband/django-auditlog/blob/master/.github/workflows/release.yml

Copy link
Collaborator Author

@aqeelat aqeelat Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nicoddemus Should I use this event instead?

on:
  release:
    types: [published]

https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#release

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think using on release and ditching the if statement is the best and simplest approach.
https://docs.github.com/en/actions/learn-github-actions/contexts#github-context

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never used this option, but seems reasonable to require a release rather than a direct tag.

Other repos might do the other way around: push a tag, and one of the things done at the end of the workflow after uploading the package to PyPI is creating a release.

Copy link
Member

@graingert graingert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3.7 is eol

@nicoddemus
Copy link
Member

Just occurred to me we should have a RELEASING.rst file (or similar) in the root which documents how the release process works. Nothing fancy mind you, see pytest-mock for example.

Copy link
Member

@flub flub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that a RELEASING.rst file would be great.

But this looks great! Thanks for looking into it.

.github/workflows/release.yaml Outdated Show resolved Hide resolved
if: github.repository == 'pytest-dev/pytest-timeout'

permissions:
id-token: write # IMPORTANT: this permission is mandatory for trusted publishing
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the publish action recommends building without this permission to minimise access to this permission. I guess that means adding a separate job for the building and passing the build output along as artefacts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If @aqeelat wants to go with that route, see how pytest-mock does it: https://github.com/pytest-dev/pytest-mock/blob/fcde66b13d09cb5507fe7e4f35e171adaf22994d/.github/workflows/deploy.yml#L13-L25

(we also build the package using @hynek's excellent build-and-inspect-python-package action).

@flub
Copy link
Member

flub commented Oct 19, 2023

since pypi seems to recommend an environment for this, i've created a release environment. can you set this deploy to use that environment as well?
https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment#using-an-environment

Copy link
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of minor suggestions, very nice to see Trusted Publishing being used here :)

.github/workflows/release.yaml Outdated Show resolved Hide resolved
.github/workflows/release.yaml Outdated Show resolved Hide resolved
flub and others added 2 commits October 23, 2023 20:13
pytest-dev is not going to fork this and means it's more re-usable

Co-authored-by: Hugo van Kemenade <[email protected]>
Otherwise we have to keep changing this.

Co-authored-by: Hugo van Kemenade <[email protected]>
@flub flub deleted the branch pytest-dev:master February 8, 2024 19:08
@flub flub closed this Feb 8, 2024
@flub
Copy link
Member

flub commented Feb 8, 2024

I switched the repo branch from master to main and somehow this got auto-closed. I also can't edit the target branch anymore in the github UI nor re-open it.

@aqeelat could you please re-open this PR? It would be really nice to have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants